-
Notifications
You must be signed in to change notification settings - Fork 397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Perf: Pairing on BN254 using direct Fp12 extension and non-native Eval()
#1339
Conversation
Suggested edit: diff --git a/std/algebra/emulated/sw_bn254/pairing.go b/std/algebra/emulated/sw_bn254/pairing.go
index fdfedafe..d2e95540 100644
--- a/std/algebra/emulated/sw_bn254/pairing.go
+++ b/std/algebra/emulated/sw_bn254/pairing.go
@@ -24,7 +24,6 @@ type Pairing struct {
curve *sw_emulated.Curve[BaseField, ScalarField]
g2 *G2
bTwist *fields_bn254.E2
- g2gen *G2Affine
}
func NewGTEl(a bn254.GT) GTEl {
@@ -82,15 +81,6 @@ func NewPairing(api frontend.API) (*Pairing, error) {
}, nil
}
-func (pr Pairing) generators() *G2Affine {
- if pr.g2gen == nil {
- _, _, _, g2gen := bn254.Generators()
- cg2gen := NewG2AffineFixed(g2gen)
- pr.g2gen = &cg2gen
- }
- return pr.g2gen
-}
-
// Pair calculates the reduced pairing for a set of points
// ∏ᵢ e(Pᵢ, Qᵢ).
//
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the suggested edits for removing deadcode.
Looks good to me, I didn't go through the formulas, but as the tests are functioning, then I think everything is valid. See only comments about removed examples and tests.
One thing I would do is to create isomorphism maps between the tower and direct extension as explicit methods and use them, as imo it makes it a bit clearer to see what extension is used where.
And I think it would be better to wait for #1340 to be merged to ensure that there are no failing tests for edge cases (which I understood you bypassed?)
That makes sense. I pushed a few comments that address your review points. We can wait for #1340. |
Yup, I agree that with different extensions computing pairing makes less sense and we'd mostly be interested in pairing check (or multi-pairing check). I guess we can keep it currently as is, but have documentation update that it may not be efficient. |
@ivokub PR is ready now! |
Suggested edit: diff --git a/std/algebra/emulated/sw_emulated/point.go b/std/algebra/emulated/sw_emulated/point.go
index 10b27de3..7fce40b4 100644
--- a/std/algebra/emulated/sw_emulated/point.go
+++ b/std/algebra/emulated/sw_emulated/point.go
@@ -214,13 +214,15 @@ func (c *Curve[B, S]) AssertIsOnCurve(p *AffinePoint[B]) {
selector := c.api.And(c.baseApi.IsZero(&p.X), c.baseApi.IsZero(&p.Y))
b := c.baseApi.Select(selector, c.baseApi.Zero(), &c.b)
- left := c.baseApi.Mul(&p.Y, &p.Y)
- right := c.baseApi.Eval([][]*emulated.Element[B]{{&p.X, &p.X, &p.X}, {b}}, []int{1, 1})
- if c.addA {
- ax := c.baseApi.Mul(&c.a, &p.X)
- right = c.baseApi.Add(right, ax)
+ mone := c.baseApi.NewElement(-1)
+
+ var check *emulated.Element[B]
+ if !c.addA {
+ check = c.baseApi.Eval([][]*emulated.Element[B]{{&p.X, &p.X, &p.X}, {b}, {mone, &p.Y, &p.Y}}, []int{1, 1, 1})
+ } else {
+ check = c.baseApi.Eval([][]*emulated.Element[B]{{&p.X, &p.X, &p.X}, {&c.a, &p.X}, {b}, {mone, &p.Y, &p.Y}}, []int{1, 1, 1, 1})
}
- c.baseApi.AssertIsEqual(left, right)
+ c.baseApi.AssertIsEqual(check, c.baseApi.Zero())
}
// AddUnified adds p and q and returns it. It doesn't modify p nor q.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Now looks very nice. I also found one optimization for AssertIsOnCurve where we can bring the left hand side to the right and then do only a single eval.
Description
Similar to #1312 but for BN254.
This PR refactor methods in
fields_bn254
andsw_bn254
using a direct Fp12 extension and non-nativeEval()
introduced in #1299.TODO:
Karabina's cyclotomic squarings.Torus-based arithmetic over direct Fp6 extension.Type of change
How has this been tested?
How has this been benchmarked?
Some circuits proved in a in a BN254-PLONK:
e(a,b)
e(a,b) * e(c,d) == 1
*ECPAIR precompiles include G2 subgroup membership.
Checklist:
golangci-lint
does not output errors locally